fix(bin): retire stalled owned watcher child with bounded TERM/KILL - #2320
fix(bin): retire stalled owned watcher child with bounded TERM/KILL#2320mayankasthana wants to merge 16 commits into
Conversation
…h handling beacon
…platform divergence
SIGKILL is never held pending for a stopped process on Linux: the bounded retirement's KILL kills the stopped watcher immediately, the arm's wait reaps it before the expected-pid hardening runs, and the retirement takes the released-lock shape (stale-beacon-retired), not the release-failed shape. The old Linux case block asserted the opposite and failed deterministically on the ubuntu runner; the platform-gated block had never run during macOS local validation.
|
Speaking as Kun's firstmate: Scheduled 3:10pm PT 8/23 pass. VISION.md read in full from current main VISION (inspected the stalled-owned-child retirement in Class: corrective. Security: none. No workflow-file / secret / injection risk. TERM/KILL is scoped to the arm-owned watcher process group after a stale-beacon predicate; fail-loud, not silent. Overlap / HOLD: CI / NM: HEAD Workflows: already approved historically (CI completed SUCCESS on 2026-08-17). Run IDs: 32033358684 (CI), 32033358622 (Require no-mistakes). No pending first-time-fork approval. What would help this PR land: rebase onto current Land-eligible rec: NO (merge conflicts; 63 behind; NM attestation mismatch; stale CI vs current main). Captain-flag NOW: no. Waiting on the author to rebase off current main, clear the conflicts, and re-stamp no-mistakes for the new HEAD. Not a captain-decision hold. |
…talled-child-retiremen-5d # Conflicts: # bin/fm-watch.sh # docs/watcher-continuity.md
…er-continuity docs
mayankasthana
changed the title
Confidence Score: 4/5The PR does not yet appear safe to merge because stale-lock cleanup can still delete a concurrently acquired healthy successor lock. The expected owner is checked before, rather than atomically during, lock removal; the clear-stale-lock transition can therefore remove a replacement watcher’s live lock in the intervening race window. Files Needing Attention: bin/fm-watch-arm.sh and bin/fm-wake-lib.sh Reviews (4): Last reviewed commit: "no-mistakes: apply CI fixes" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Speaking as Kun's firstmate: Scheduled 11:10am PT 8/24 pass. VISION.md read in full from current main VISION (re-inspected stalled-owned-child retirement in Class: corrective. Security: none. No workflow-file / secret / injection. TERM/KILL is scoped to the arm-owned watcher process group after a stale-beacon predicate; fail-loud. Greptile still flags a successor-lock race in Overlap: THIS HEAD vs last pass: conflicts cleared. GitHub mergeable=MERGEABLE, mergeStateStatus=UNSTABLE. ahead 14 / behind 0 (was ahead 10 / behind 63). I will not resolve conflicts — they are already gone — and I will not conflict-fix via cloud agent because the PR is still not otherwise auto-merge-ready. CI / NM: HEAD Workflows approved this pass: CI 32757334876, Require no-mistakes 32757334813. Not green at comment time. Land-eligible rec: NO (NM attestation mismatch vs THIS HEAD; CI not yet green). Captain-flag NOW: no. Waiting-on-author to regenerate |
|
Speaking as Kun's firstmate: recirc. Conflicts look cleared vs last pass (now MERGEABLE). Still not auto-eligible. class=corrective. Bounded TERM/KILL of an arm-owned stalled watcher child after a stale-beacon predicate. Restores intended liveness. VISION.md: restart-as-non-event aligns. Honest interface aligns. Scripts align. Authority n/a. Spine aligns. Vendor aligns. Scope aligns. This HEAD: Overlap: Waiting on author for a HEAD-matching attestation and green CI on this SHA. Not a captain-decision hold. |
2 participants
Intent
Ship PR #2320: retire a stalled owned watcher child with a bounded TERM/KILL sequence, resolving the conflict against main via the no-mistakes pipeline.
What Changed
bin/fm-watch-arm.sh: after an arm forks and confirms an owned watcher, follow liveness via the same identity-bound beacon predicate instead of a rawwaiton a live PID. When that owned child reaches the shared stale-beacon grace, an arm-owned watchdog retires it with a bounded TERM then KILL to its isolated process group (defaultFM_WATCH_STALL_RETIRE_TIMEOUT=2), publishes the existing watcher-down recovery episode, removes the stale lock only once the child is dead and still matches the expected PID, and exits with a typed failure so a persistent adapter can retry without a primary-session restart. Bounded stand-down/retirement paths also cover a won-race child and a lost-race child that stalls before exiting, so no stall can block the arm inwaitforever.bin/fm-watch.sh: while a successor is being handled, a new bounded loop keeps touching the watcher beat and re-snapshots the downtime marker until the pending-downtime transition clears (or a bounded budget exhausts and flagsWATCHER_RECOVERY_PENDING).Tests and docs: expand
tests/fm-watch-arm.test.shandtests/fm-watcher-lock.test.sh(including a real-process SIGSTOP counterfactual covering the bounded retirement contract), add.claude/hooks/to.gitignore, and updatedocs/configuration.mdanddocs/watcher-continuity.mdfor the newFM_WATCH_STALL_RETIRE_TIMEOUTknob and the bounded retirement outcomes.Risk Assessment
Testing
Exercised the stalled-owner retirement change with the dedicated arm suite (all pass, including the new lost-race bounded stand-down regression), the modified SIGSTOP retirement test in isolation (passes), and a live end-to-end demo using a real arm and a SIGSTOP'd owned watcher that captures the user-visible bounded TERM/KILL retirement, typed FAILED line, published recovery state, and released lock. One pre-existing host-level lock-concurrency test failure on both base and target was identified (unmodified code path, unrelated to this change) and reported as informational.
Evidence: End-to-end stalled-watcher retirement transcript
Source: End-to-end stalled-watcher retirement transcript
command: real fm-watch-arm.sh forks owned watcher; SIGSTOP makes it live-but-stalled; watchdog retires group. arm status: watcher: started pid=95675 (beacon fresh) watcher: FAILED - watcher pid=95675 stopped advancing its beacon for 1s; retired the stalled cycle and released stale ownership for bounded recovery arm exit code: 1 recovery state published: pending:downtime:95655.1787589187.0JeLd1 ledger: ... reason=stale-beacon-retired ... signal=KILL ... lock_before=pid:95675|... lock_after=pid:none|identity:none successor=none own watcher alive after retirement: dead (retired)Evidence: fm-watch-arm test suite results
Evidence: Modified stopped-watcher retirement test result
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-watch-arm.sh:591- The arm's signal-interrupt path now escalates TERM→SIGKILL of the whole watcher process group. Previously handle_arm_signal only TERM'd the owned child and waited for it. Now cleanup_child -> retire_watch_child sends TERM and then, if the child hasn't exited within STALL_RETIRE_TIMEOUT+1s (~3s), SIGKILLs the entire isolated group (both the lock holder and any descendants). This is a deliberate but behavior-widening consequence of 'bounded retirement': it fires on a routine arm interrupt (e.g. a predecessor arm torn down during handoff, as exercised by tests that TERM an arm and expect exit 143 with arm-interrupted classification) even when the child's beacon is fresh. Impact is low because the watcher traps TERM and persists recovery via its own EXIT trap well within the bound — SIGKILL only lands on a genuinely hung/ignoring-foreground-sleep child, in which case it couldn't persist recovery anyway and the refusal path keeps the lock. Worth confirming this escalation on the interrupt path (as distinct from the stale-beacon path) is intended.bin/fm-watch-arm.sh:704- Minor reaping gap in wait_owned_child's stale branch: if the loop breaks on the deadline while the child is still running (WATCH_CHILD_RC=124), the arm sends KILL (line 714) and returns 1 without a wait/reap, leaving the child as a brief zombie or, in the refusal case, a live orphan holding the lock until the arm process exits and it is reparented. Self-heals on exit and matches the documented refusal shape, so not a blocker — just noting the lack of an explicit reap on this path versus the siblingelif waitbranch.tests/fm-watcher-lock.test.sh:877- Pre-existing unrelated failure: test_lock_single_winner_under_concurrency (tests/fm-watcher-lock.test.sh) fails on this host — 2-3 'lock winners' instead of 1 — on BOTH the base commit (038d0f7) and the target commit. It exercises only the unmodified fm_lock_try_acquire primitive (bin/fm-wake-lib.sh, tests/lib.sh, tests/wake-helpers.sh are byte-identical between base and target; the change touches only fm-watch-arm.sh/fm-watch.sh/docs). This is host-contention flake out of scope for this change and not fixable in this test phase. Because fail() aborts the whole file, this failure blocks the rest of the suite when run as a full file; the change's own tests were validated in isolation and pass. Not caused by, and not actionable for, this PR.bash tests/fm-watch-arm.test.sh (full suite: all 16 pass, incl. new test_lost_race_child_stand_down_is_bounded and modified test_rearm_resurfaces_durable_queue_and_remote_open_decision)bash tests/fm-watcher-lock.test.sh (full run aborts at pre-existing test_lock_single_winner_under_concurrency failure, host-unrelated)driver-concurrency.sh: test_lock_single_winner_under_concurrency in isolation on target (2 winners x5) and on base worktree 038d0f7 (2-3 winners x3) - confirms pre-existing, unrelateddriver-stopped-watcher.sh: modified test_stopped_watcher_is_retired_and_rearms_without_session_restart run in isolation (PASS: retires live stale watcher, releases recovery state, preserves healthy successor)demo-retire-e2e.sh: real fm-watch-arm.sh forks owned watcher, SIGSTOP makes it live-but-stalled, watchdog retires group (TERM->KILL) within bound, arm exits 1 with typed FAILED line, publishes pending:downtime state, ledger records reason=stale-beacon-retired signal=KILL lock_after=pid:none, watcher dead✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.